-
Notifications
You must be signed in to change notification settings - Fork 24.9k
Correctly implement transform: matrix with 2d matrices #53860
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@cortinico has exported this pull request. If you are a Meta employee, you can view the originating diff in D82836192. |
Summary: Fixes facebook#53639 I've realized that our docs and validation logic states that we do support 3x3 matrixes (for 2d transforms). However they're not properly processed as the values are just copied into a 4x4 matrix. This can be verified by applying the 3x3 identity matrix on any transform. I'm fixing it by correctly populating the 4x4 matrix getting the values from the 3x3 matrix in input. Changelog: [General] [Fixed] - 9-element (2d) transform matrix are not correctly working Reviewed By: christophpurrer Differential Revision: D82836192
289dacf to
ab0b82c
Compare
|
@cortinico has exported this pull request. If you are a Meta employee, you can view the originating diff in D82836192. |
|
This pull request has been merged in ce243df. |
|
This pull request was successfully merged by @cortinico in ce243df When will my fix make it into a release? | How to file a pick request? |
| } else if (numbers.size() == 9) { | ||
| // We need to convert the 2d transform matrix into a 3d one as such: | ||
| // [ | ||
| // x01, x02, 0, x03 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be x00, x01, 0, x02 if we start numbering from 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be
x00, x01, 0, x02if we start numbering from 0
yeah I believe this is fixed already on main, I followed up in a separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, my bad, sorry, all good
Summary:
Fixes #53639
I've realized that our docs and validation logic states that we do support
3x3 matrixes (for 2d transforms). However they're not properly processed
as the values are just copied into a 4x4 matrix.
This can be verified by applying the 3x3 identity matrix on any transform.
I'm fixing it by correctly populating the 4x4 matrix getting the values from the 3x3 matrix in input.
Changelog:
[General] [Fixed] - 9-element (2d) transform matrix are not correctly working
Differential Revision: D82836192